FIX: out of range exception when pressing undo after creating and editing a new control scheme (ISXB-1607) #2237
Conversation
…ed by Undoing the creation of it via ctrl+z.
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2237 +/- ##
===========================================
- Coverage 68.14% 68.13% -0.02%
===========================================
Files 367 367
Lines 53685 53700 +15
===========================================
+ Hits 36585 36587 +2
- Misses 17100 17113 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Darren-Kelly-Unity
changed the title
| { | ||
| try | ||
| { | ||
| await Task.Delay(1); |
There was a problem hiding this comment.
This await Task.Delay(1) raises warning flags to me. What guarantees this is valid to execute 1 second from now?
- Why does it have to be delayed?
- It looks like this runs async but code being accessed do not seem to be thread safe?
- Is there a logical condition not based on time when this is valid to execute?
There was a problem hiding this comment.
You are right, it was something I added at the end to solve an exception but it's not the correct way to do this.
I have now resorted to just closing the window when an undo is triggered if it's open.
This has one side effect though that it will trigger the view to close in a case where it might not need to, definitely less bad than all the exceptions it could cause previously.
The best solution that I could not figure out a way to do would be to add the opening of the control scheme editor popup to the undo stack so that pressing undo would close it, I did find a way to possibly do this but it would add a good amount of complexity and would be quite hacky.
I chose not to go this route in the end as the undo api is not really designed to be used with things that are not Objects/GameObjects.
|
|
||
| s_OnPasteCutElements.Add(this); | ||
|
|
||
| Undo.undoRedoPerformed += CheckForInvalidControlSchemeInOneFrame; |
There was a problem hiding this comment.
Maybe I am misunderstanding this PR/issue so I want to check whether this problem is if you undo text editing while in text editing mode? Such editing should not have been applied yet to any serializedProperty right or am I misunderstanding this issue?
There was a problem hiding this comment.
It's a bit different than that, basically the last undo when opening a control scheme to edit is the creation of the asset.
Therefore undoing while editing the name actually deletes the asset.
This of course causes exceptions when inside the edit window if you try to alter the asset or even try to close it.
My solution I have landed on for now is to close the control scheme view when this happens and revert to selecting the all controls control scheme now instead.
Description
You could cause multiple null or out of range exceptions if you press undo while editing a control scheme.
This is caused by undo causing the control scheme to be removed while you still have the edit window open.
Now the edit window will close immediately if you press undo while it's open.
Ticket:
https://jira.unity3d.com/browse/ISXB-1607
Video of fix (Usually undoing when editing the new control scheme would throw an exception / it could also cause other exceptions if the window was left open, therefore I now close it when this happens to avoid future issues):
Screen.Recording.2025-10-07.at.15.06.14.mov
Testing status & QA
Tested manually all the cases I could find until all exceptions were gone. There was multiple paths to create exceptions due to the asset being removed via undo.
Overall Product Risks
Please rate the potential complexity and halo effect from low to high for the reviewers. Note down potential risks to specific Editor branches if any.
Comments to reviewers
Please describe any additional information such as what to focus on, or historical info for the reviewers.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.After merge: